Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a PR template and issue template #394

Merged
merged 16 commits into from
Oct 24, 2024

Conversation

OussamaSaoudi-db
Copy link
Collaborator

@OussamaSaoudi-db OussamaSaoudi-db commented Oct 14, 2024

What changes were proposed in this pull request?

This PR adds a pull request template and issues template to the repository.
The issues template is adapted from Apache Datafusion's issue template: https://github.com/apache/datafusion/.
This pull request template is adapted from Apache Spark's pull request template: https://github.com/apache/spark

Why are the changes needed?

A pull request template encourages open source contributions to clearly state their purpose and approach. It explicitly calls out user facing changes and explains why they are needed. It also encourages all contributions to be well tested. In the event testing was difficult or impossible, this exposes testing pain points can be addressed.

Issues template encourages community members and users to give all the details that would help bugs get fixed and feature requests implemented. Requiring more details from issues also helps newcomers to the project get more context to get started contributing.

Does this PR introduce any user-facing change?

No

How was this change tested?

No way to test pull request templates or issue templates.

Copy link

codecov bot commented Oct 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.94%. Comparing base (e48d238) to head (1e829ef).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #394   +/-   ##
=======================================
  Coverage   78.94%   78.94%           
=======================================
  Files          52       52           
  Lines       10756    10756           
  Branches    10756    10756           
=======================================
  Hits         8491     8491           
  Misses       1796     1796           
  Partials      469      469           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@OussamaSaoudi-db OussamaSaoudi-db marked this pull request as ready for review October 14, 2024 21:24
@OussamaSaoudi-db OussamaSaoudi-db changed the title Add a PR template to state rationale, breaking changes, and testing Add a PR template and issue template Oct 14, 2024
Copy link
Collaborator

@nicklan nicklan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Just a few wording suggestions.

.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@zachschuermann zachschuermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just lots of wording. but generally looks good - thanks! I'm mostly advocating for making it simpler (I think there is a balance between not enough/too much structure for these things)

.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
@zachschuermann
Copy link
Collaborator

Also can we make the 'user-facing changes' section of the PR description hidden by default with a comment that says 'uncomment if user-facing changes'? that way we don't just have tons of PRs with 'user-facing changes?' 'No.'

Copy link
Collaborator

@zachschuermann zachschuermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice - few comments :)

.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@zachschuermann zachschuermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm i vote we merge and iterate

.github/pull_request_template.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@nicklan nicklan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! thanks

@OussamaSaoudi-db OussamaSaoudi-db merged commit f5d0a42 into delta-io:main Oct 24, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants